Validate key type in _new_public_key_x509_der() on 3.x#173
Merged
toddr merged 3 commits intocpan-authors:mainfrom Apr 23, 2026
Merged
Conversation
On OpenSSL 3.x, d2i_PUBKEY_bio() accepts any key type (EC, DSA, etc.) but _new_public_key_x509_der() has no type validation, unlike _load_rsa_key() which checks EVP_PKEY_get_base_id(). This test demonstrates the gap: an EC public key in X.509 DER format is silently accepted, creating a corrupt rsaData object. The test intentionally fails against the current code to document the vulnerability before the fix is applied. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
On OpenSSL 3.x, d2i_PUBKEY_bio() accepts any key type (EC, DSA, etc.), unlike the pre-3.x d2i_RSA_PUBKEY_bio() which only accepts RSA keys. Without validation, a non-RSA DER key would be stored in the rsaData struct, leading to corrupt state and confusing failures on subsequent operations. Add the same EVP_PKEY_get_base_id() != EVP_PKEY_RSA guard already present in _load_rsa_key() (line 401) to give a clear "not an RSA key" error at load time. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Member
|
@toddr-bot the check needs to be: like($@, qr/not an RSA key|ASN1|expecting an rsa key/i, so that it works on Bullseye |
On pre-3.x OpenSSL, d2i_RSA_PUBKEY_bio() rejects EC keys with "expecting an rsa key" rather than "not an RSA key" (our croak) or an ASN1 error. Add this variant to the regex so the test passes across all supported OpenSSL versions. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
timlegge
approved these changes
Apr 23, 2026
toddr
reviewed
Apr 23, 2026
| #if OPENSSL_VERSION_NUMBER >= 0x30000000L | ||
| if (EVP_PKEY_get_base_id(pkey) != EVP_PKEY_RSA) { | ||
| EVP_PKEY_free(pkey); | ||
| croak("The key loaded is not an RSA key"); |
Member
There was a problem hiding this comment.
Does this leak any SVs as we croak here?
toddr
approved these changes
Apr 23, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Add
EVP_PKEY_get_base_id() != EVP_PKEY_RSAvalidation to_new_public_key_x509_der()for OpenSSL 3.x builds.Why
On OpenSSL 3.x,
d2i_PUBKEY_bio()accepts any key type (EC, DSA, etc.), unlike the pre-3.xd2i_RSA_PUBKEY_bio()which inherently rejects non-RSA keys. Without validation, a non-RSA DER key is stored in thersaDatastruct, leading to a corrupt object that fails unpredictably on subsequent operations.The PEM-based
_load_rsa_key()already has this guard (line 401) — this closes the same gap for the DER code path.How
Added the same
#if OPENSSL_VERSION_NUMBER >= 0x30000000L/EVP_PKEY_get_base_id()pattern from_load_rsa_key(). On mismatch, frees the key and croaks with "The key loaded is not an RSA key".Two commits:
_new_public_key_x509_der()directlyTesting
t/der.t(tests 23-24): EC DER key rejected with clear error🤖 Generated with Claude Code
Quality Report
Changes: 5 files changed, 72 insertions(+), 41 deletions(-)
Code scan: clean
Tests: passed (OK)
Branch hygiene: clean
Generated by Kōan post-mission quality pipeline